`_MeshIndexSet` first pass by trexfeathers · Pull Request #7149 · SciTools/iris

trexfeathers · 2026-06-10T10:27:39Z

No description provided.

…ndex-set

trexfeathers · 2026-06-10T11:11:00Z

pre-commit.ci autofix

trexfeathers · 2026-06-10T14:36:38Z

@stephenworsley please can you review the approach I have used with this code, accepting that various TODOs are deferred to #7151. Thanks!

stephenworsley · 2026-06-10T14:59:59Z

+        raise NotImplementedError()
+
+    def is_view_of(self, other: MeshXY) -> bool:
+        """Whether this instance is either itself, or a view of the given :class:`MeshXY`.


Have you considered how this ought to behave when you've sliced or indexed a _MeshIndexSet? If I'm interpreting the code correctly, it looks like you ought to end up with another smaller _MeshIndexSet. In such a case, I could imagine you might want this method to also return True.

I also wonder if it might be worth considering two different methods, one for comparing a _MeshIndexSet to a MeshXY, another for comparing two _MeshIndexSets. I haven't entirely thought through which contexts you might expect to call these methods so I've not come to a conclusion on this myself yet.

This is not intended to be possible, and I've taken some steps to avoid it happening:

iris/lib/iris/mesh/components.py

Lines 3619 to 3630 in ec132b0

case _MeshIndexSet():

# Should not base an index set on another index set - base

# on the original mesh instead.

# TODO: do we need to double-check that self.location

# matches mesh_index_set.location? Any other matching to

# check too?

mesh_index_set = mesh

kwargs = dict(

mesh=mesh_index_set.mesh,

location=mesh_index_set.location,

indices=mesh_index_set.indices[keys],

)

trexfeathers · 2026-06-10T16:01:07Z

+        result = [self.to_MeshCoord(location=location, axis=ax) for ax in self.AXES]
+        return tuple(result)
+
+    def is_view_of(self, other: "MeshXY") -> bool:


Offline conversation with @stephenworsley: is_view_of is not necessary, and any value it might offer is offset by potential confusion. We should remove it.

trexfeathers · 2026-06-10T16:38:33Z

        result_dict = {k: v for k, v in self._members.items() if id(v) in result_ids}
        return result_dict

+    def index(


Offline conversation with @stephenworsley: this concept of mutability only really extends to altering the membership of the Manager, there is no decent way ¹ to protect the coordinates/connectivities themselves from modification, since users are free to assign those to their own variables, and/or to modify the arrays within them.

If we're comfortable with this 'insecurity', then we could consider generating new constituent coordinates/connectivities that explicitly share the same NumPy array as the ones on the original Mesh (as opposed to directly using the slicing API, since that explicitly creates a copy). That would allow the MeshIndexSet to be a true view, with live updating, potentially avoiding the weirdness of re-indexing every time a Manager is requested.

Footnotes

I need to read up on Ensure MeshCoord points and bounds always agree with the Mesh (while keeping Mesh mutable) #4757 ↩

Didn't turn out as expected - indexing arrays using another array of integers NEVER returns a view - but I've got something working.

a6542ee

trexfeathers · 2026-07-06T17:32:57Z

~~Note that currently printing MeshCoords based on _MeshIndexSet is very slow. Computing points and bounds seems OK, which is the bit I was originally worried about.~~

EDIT: fixed by ab688cf

stephenworsley · 2026-07-08T14:55:47Z

+    def cube_dims(self, cube):
+        raise NotImplementedError()
+
+    def as_mesh(self) -> MeshXY:


It's worth considering if we want to realise the data when we call this. This ought to be the most "robust" way of handling this operation. There's maybe a case for keeping this lazy, in order to minimise memory use, but I'm not convinced this is the correct behaviour or even something we can properly implement.

stephenworsley

I've recorded some stuff to follow up later, but this all looks good in principle. A good start for the feature branch, good work!

stephenworsley · 2026-07-08T16:19:23Z

+                if connectivity.location_axis == 1:
+                    new_values = new_values.T
+                if connectivity.start_index == 1:
+                    new_values = new_values + 1


Is there a reason this isn't just new_values += connectivity.start_index?

stephenworsley · 2026-07-09T10:20:07Z

+
+                # Map node indices in "values" to their new zero-based positions
+                #  in "node_indices".
+                order = node_indices.argsort()


It looks like this code is focused on ***_node_connectivity. I suspect this might fail with something like face_edge_connectivity. It may be worth just throwing an error in such cases for the time being. With that said, I don't think this is a blocker to moving forward with work on this feature branch.

IIRC, node-based operations are special. Edge- and face-based operations are much easier to calculate; that's why the code looks the way it does?

stephenworsley · 2026-07-09T10:34:26Z

+            if connectivity is not None:
+                indexing = indices_dict[connectivity.location]
+            if indexing is not None:
+                new_values = connectivity.indices_by_location(


Doesn't need addressing right now, but I think this bit of code could definitely benefit from thinking about making these variable names clearer. In particular, I think this could benefit from being something other than new_values.

stephenworsley · 2026-07-09T10:43:50Z

+                order = node_indices.argsort()
+                old_sorted = da.from_array(node_indices[order])
+                new_ids_sorted = da.arange(len(node_indices))[order]
+                positions = functools.partial(da.searchsorted, old_sorted)


new_ids_sorted and positions should be the same for every loop so we should probably be calculating these outside the for loop. Maybe worth adding a bit of logic to ensure they aren't called unnecessarily.

stephenworsley · 2026-07-09T11:02:06Z

+                    array=new_ids_sorted,
+                    indexing=new_values,
+                    pre_index=positions,
+                )


I think I finally got my head around what this bit of logic does. While there's bits I think can be tidied up, I'm happy with what this does in principle.

stephenworsley

These changes look good also! This can get merged now.

@trexfeathers

* `_MeshIndexSet` first pass (#7149) * Use Monotonicity error in Cube indexing, plus test coverage. * Implement MeshIndexSetMetadata. * Changes to cube.mesh type hinting. * Create experimental/mesh_coord_indexing.py . * Implement MeshIndexSet. * Render _MeshIndexSet in docs. * Easy CI fixes. * style: pre-commit fixes * Add TODO for later type hinting. * Missing sphinx-needs item. * Remove linkable UGRID footnotes to avoid duplication warning. * Fix doctest. * TODO comments. * Fix doctest. * TODO comment. * Fix doctest. * Achieve an accurate view of Mesh via Dask arrays and timestamps. * Remove Mesh.is_view_of. * Fixes after rough testing. * Prevent overzealous updates. * Disconnect new meshes from original meshes. * Review suggestions. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Refactor `_MeshIndexSet` for true lazy support (#7209) * Refactor _MeshIndexSet for true lazy support. * Update lib/iris/mesh/components.py * Resolve MyPy failures. * Review comments. * Better naming. * Correct mathematical terminology. * (Partially) Finalise meshindexset (#7207) * remove unused import * made _MeshXYMixin get and set state abstract * bonus comments * Add _MeshCoordinateManager as parent ot 1d and 2d * typing with _MeshConnectivityManagerBase * fix typing generator * accounting for dask array in _MeshConnectivityManagerBase indexed (to be discussed) * hacky way to fix test * Sequence check for MeshCoord * _MeshCoordinateManagerBase saves and gets _view_message with state * Raise exception when MeshXY.from_coords is called with MeshCoords * validation to _MeshIndexSet __init__ * Typing _MeshIndexSet. Other areas touched for mypy * Raise exception when saving _MeshIndexSet. Could possibly be done earlier * Added _MeshIndexSet._NOT_IMPLEMENTED * using match-case for _MeshIndexSet._calculate_node_indices and removing the null case because it is handled in the __init__ * Prevent mismatch of location in MeshCoord.__init__ when supplied a _MeshIndexSet * Further typing of _MeshXYMixin * post merge numpy fixes * Added forgotten bonus comment * fixed logic mistake in summary * updating remove_duplicate_nodes in tests to deal with `None`s * Update _MeshIndexSet init validation to use isinstance * remove finished todo * remove completed todos * remove unused import * update _calculate_node_bool_index to use match case * fixed ambiguous truthy and reformatted indexed * updated _MeshIndexSet saving is not yet supported error message * Remove completed todos * fixed from_coords type check and updated error message * improverd _MeshIndexSet.__init__ error messages * removed completed todo * Updated MeshCoord.__getitem__ error message * refactor _MeshCoordinateManagerBase.indexed * Cope with scalar indexing, plus view_message fix. * Updated expected exception messages * Made _MeshIndexSet.__init__ indices more lenient --------- Co-authored-by: Martin Yeo <martin.yeo@metoffice.gov.uk> * Docstrings for MeshIndexSet work (#7213) * Docstrings for MeshIndexSet work. * More explication. * Indicate default `start_index`. Co-authored-by: pt331 <144435193+pt331@users.noreply.github.com> --------- Co-authored-by: pt331 <144435193+pt331@users.noreply.github.com> * Fixes based on early testing. (#7216) * Tests for indexing meshes (#7218) * Barebones tests * assertion fix and allowing for single value index * fixed tests/stock/mesh topology_dimension * Update as_mesh to deal with locations not existing * Copied as_mesh changes from unit tests branch * Applied equality changes offered by @trexfeathers * Remove cube_mesh_node from test_subset_indexing_new_mesh test * updating to full tests - test_subset_indexing_mesh_index_set fails now * Fixed looking at the wrong part of cube and _MeshIndexSet test * Unit tests for MeshIndexSet. (#7219) * Unit tests for MeshIndexSet. * Convert existing tests to use MonotonicityError. * Unit tests for mesh_coord_indexing. * Wider topology and location coverage. * Tests for printing. * Indexing improvements. * Test lazy cases. * Increased test coverage. * Increased coverage. * Increased coverage. * Fix test typo. * Mark _MeshIndexSetMetadata as private. * eq unit tests. * Make _MeshIndexSet hashable as intended. * More thorough varied mesh testing. * MeshIndexSet What's News (#7225) * What's New entries for MeshIndexSet. * Extra experimental note. * Review comments. * Fix pre-commit failures. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: pt331 <144435193+pt331@users.noreply.github.com>

trexfeathers added 7 commits June 9, 2026 15:00

Use Monotonicity error in Cube indexing, plus test coverage.

52bcda4

Implement MeshIndexSetMetadata.

848e822

Changes to cube.mesh type hinting.

6b151f5

Create experimental/mesh_coord_indexing.py .

8fc2e79

Implement MeshIndexSet.

40779c4

Render _MeshIndexSet in docs.

c093fd2

Merge remote-tracking branch 'upstream/FEATURE_index_set' into mesh-i…

f1b54da

…ndex-set

trexfeathers added Feature: UGRID Type: Feature Branch Highlight this for a feature branch labels Jun 10, 2026

Easy CI fixes.

e1a220f

pre-commit-ci Bot and others added 6 commits June 10, 2026 11:11

style: pre-commit fixes

8e5722b

Add TODO for later type hinting.

8b7ec5e

Missing sphinx-needs item.

c20d53c

Remove linkable UGRID footnotes to avoid duplication warning.

70cb63c

Fix doctest.

3724c3e

TODO comments.

047c40d

trexfeathers mentioned this pull request Jun 10, 2026

Create MeshIndexSet class #6121

Closed

4 tasks

trexfeathers added 2 commits June 10, 2026 15:00

Fix doctest.

dcbb5e0

TODO comment.

36dd627

trexfeathers mentioned this pull request Jun 10, 2026

Finalise _MeshIndexSet work from #7149 #7151

Closed

21 tasks

trexfeathers requested a review from stephenworsley June 10, 2026 14:36

Fix doctest.

ec132b0

trexfeathers mentioned this pull request Jun 10, 2026

Index MeshCoord using new MeshXY or _MeshIndexSet #6127

Closed

3 tasks

trexfeathers linked an issue Jun 10, 2026 that may be closed by this pull request

Create MeshIndexSet class #6121

Closed

4 tasks

trexfeathers mentioned this pull request Jun 10, 2026

Write tests for UGRID Location Index Set data model #7152

Closed

trexfeathers marked this pull request as ready for review June 10, 2026 15:02

stephenworsley reviewed Jun 10, 2026

View reviewed changes

trexfeathers commented Jun 10, 2026

View reviewed changes

scitools-ci Bot added this to 🚴 Peloton Jun 11, 2026

trexfeathers added 2 commits July 1, 2026 16:51

Achieve an accurate view of Mesh via Dask arrays and timestamps.

a6542ee

Remove Mesh.is_view_of.

000c259

trexfeathers commented Jul 1, 2026

View reviewed changes

Comment thread lib/iris/mesh/components.py Outdated

Fixes after rough testing.

2519f2f

Prevent overzealous updates.

ab688cf

stephenworsley reviewed Jul 8, 2026

View reviewed changes

stephenworsley approved these changes Jul 9, 2026

View reviewed changes

trexfeathers added 2 commits July 13, 2026 17:11

Disconnect new meshes from original meshes.

f8efd13

Review suggestions.

4b3adc5

stephenworsley approved these changes Jul 15, 2026

View reviewed changes

stephenworsley merged commit 5d1180d into SciTools:FEATURE_index_set Jul 15, 2026
15 of 19 checks passed

github-project-automation Bot moved this to Done in 🚴 Peloton Jul 15, 2026

scitools-ci Bot removed this from 🚴 Peloton Aug 13, 2026

	case _MeshIndexSet():
	# Should not base an index set on another index set - base
	# on the original mesh instead.
	# TODO: do we need to double-check that self.location
	# matches mesh_index_set.location? Any other matching to
	# check too?
	mesh_index_set = mesh
	kwargs = dict(
	mesh=mesh_index_set.mesh,
	location=mesh_index_set.location,
	indices=mesh_index_set.indices[keys],
	)

Uh oh!

Conversation

trexfeathers commented Jun 10, 2026

Uh oh!

trexfeathers commented Jun 10, 2026

Uh oh!

trexfeathers commented Jun 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trexfeathers Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trexfeathers commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenworsley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenworsley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trexfeathers Jun 10, 2026 •

edited

Loading

trexfeathers commented Jul 6, 2026 •

edited

Loading